-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Include missing reflected type in hashcode calculation #120449
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Include missing reflected type in hashcode calculation #120449
Conversation
Tagging subscribers to this area: @dotnet/area-system-reflection |
src/coreclr/System.Private.CoreLib/src/System/Reflection/RtFieldInfo.cs
Outdated
Show resolved
Hide resolved
Can we add some tests to validate that the hashcodes are different for different reflected types? (The test should try multiple types to be resilient against hash collisions.) |
src/libraries/System.Runtime/tests/System.Reflection.Tests/EventInfoTests.cs
Outdated
Show resolved
Hide resolved
Looks like NativeAOT needs the same fix. NativeAOT has a separate implementation of reflection that lives under src\coreclr\nativeaot. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a few comment regarding consistency between Equals
and GetHashCode
on the different classes.
In some cases it might be an issue on the Equals
itself and is probably out of the scope of this PR.
src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeConstructorInfo.CoreCLR.cs
Outdated
Show resolved
Hide resolved
...oreLib/src/System/Reflection/Runtime/EventInfos/NativeFormat/NativeFormatRuntimeEventInfo.cs
Outdated
Show resolved
Hide resolved
...ate.CoreLib/src/System/Reflection/Runtime/MethodInfos/RuntimeConstructedGenericMethodInfo.cs
Outdated
Show resolved
Hide resolved
...stem.Private.CoreLib/src/System/Reflection/Runtime/MethodInfos/RuntimeSyntheticMethodInfo.cs
Outdated
Show resolved
Hide resolved
.../src/System/Reflection/Runtime/PropertyInfos/NativeFormat/NativeFormatRuntimePropertyInfo.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Runtime/tests/System.Reflection.Tests/EventInfoTests.cs
Outdated
Show resolved
Hide resolved
src/coreclr/System.Private.CoreLib/src/System/Reflection/RtFieldInfo.cs
Outdated
Show resolved
Hide resolved
...oreLib/src/System/Reflection/Runtime/EventInfos/NativeFormat/NativeFormatRuntimeEventInfo.cs
Outdated
Show resolved
Hide resolved
src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeEventInfo.cs
Outdated
Show resolved
Hide resolved
src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimePropertyInfo.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
Fix incorrect runtime member info (
RuntimePropertyInfo
,RuntimeMethodInfo
,RuntimeEventInfo
,RuntimeFieldInfo
,RuntimeConstructorInfo
) hashcode calculation. Previously it did not include the reflected type, resulting in different sub classes having the same hashcode for properties declared in the base class.Fixes #114280